-
Notifications
You must be signed in to change notification settings - Fork 842
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[EuiBasicTable] Remove preventing all mouse/keyboard interactions when loading
#6543
Conversation
…ble when `loading` + remove unnecessary 11 `loading` listeners
Preview documentation changes for this PR: https://eui.elastic.co/pr_6543/ |
This reverts commit 7109b4a.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 LGTM! Tested for keyboard interaction and verified no regressions using Safari and Firefox with VoiceOver screen reader. Latest Chrome axe-core
plugin returned 0 violations.
Preview documentation changes for this PR: https://eui.elastic.co/pr_6543/ |
## Summary `eui@73.0.0` ⏩ `eui@74.0.1` --- ## [`74.0.1`](https://github.com/elastic/eui/tree/v74.0.1) **Bug fixes** - Fixed `EuiModalHeaderTitle` type errors when passed `EuiTitle` props ([#6547](elastic/eui#6547)) ## [`74.0.0`](https://github.com/elastic/eui/tree/v74.0.0) - Added the `component` prop to `EuiModalHeaderTitle`, which allows overriding the default `h1` tag ([#6530](elastic/eui#6530)) - Added the `titleProps` prop to `EuiConfirmModal`, which allows overriding the default `h1` tag ([#6530](elastic/eui#6530)) **Bug fixes** - Fixed slight row height jumping in `EuiBasicTable`s when actions with tooltips became disabled ([#6538](elastic/eui#6538)) **Breaking changes** - `EuiModalHeaderTitle` now **always** wraps its children in a `h1` tag (previously attempted to conditionally detect whether its children were raw strings or not). To change this tag type to, e.g. a more generic `div`, use the new `component` prop. ([#6530](elastic/eui#6530)) - `EuiLink` now applies `rel="noreferrer"` to all domains, including `elastic.co` ([#6535](elastic/eui#6535)) - `EuiBasicTable` no longer blocks mouse/keyboard interactions while `loading` ([#6543](elastic/eui#6543)) **CSS-in-JS conversions** - Converted `EuiBasicTable` to Emotion ([#6539](elastic/eui#6539)) - Added a new `RenderWithEuiTheme` render prop utility ([#6539](elastic/eui#6539)) --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
## Summary `eui@73.0.0` ⏩ `eui@74.0.1` --- ## [`74.0.1`](https://github.com/elastic/eui/tree/v74.0.1) **Bug fixes** - Fixed `EuiModalHeaderTitle` type errors when passed `EuiTitle` props ([elastic#6547](elastic/eui#6547)) ## [`74.0.0`](https://github.com/elastic/eui/tree/v74.0.0) - Added the `component` prop to `EuiModalHeaderTitle`, which allows overriding the default `h1` tag ([elastic#6530](elastic/eui#6530)) - Added the `titleProps` prop to `EuiConfirmModal`, which allows overriding the default `h1` tag ([elastic#6530](elastic/eui#6530)) **Bug fixes** - Fixed slight row height jumping in `EuiBasicTable`s when actions with tooltips became disabled ([elastic#6538](elastic/eui#6538)) **Breaking changes** - `EuiModalHeaderTitle` now **always** wraps its children in a `h1` tag (previously attempted to conditionally detect whether its children were raw strings or not). To change this tag type to, e.g. a more generic `div`, use the new `component` prop. ([elastic#6530](elastic/eui#6530)) - `EuiLink` now applies `rel="noreferrer"` to all domains, including `elastic.co` ([elastic#6535](elastic/eui#6535)) - `EuiBasicTable` no longer blocks mouse/keyboard interactions while `loading` ([elastic#6543](elastic/eui#6543)) **CSS-in-JS conversions** - Converted `EuiBasicTable` to Emotion ([elastic#6539](elastic/eui#6539)) - Added a new `RenderWithEuiTheme` render prop utility ([elastic#6539](elastic/eui#6539)) --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Summary
closes #5014
While converting EuiBasicTable to Emotion, I happened upon this tbody logic/ref added when EuiInMemoryTable was first introduced (#390), although unfortunately, I could not find any context as to why it was introduced/approved.
Unfortunately, I strongly feel that this behavior is problematic at worst (accessibility - keyboard users who tab into a loading table will never be able to tab off) and totally unnecessary at best (we're adding and removing 11 event listeners on loading change, for very little UX benefit).
I'm proposing we remove this, with designer approval and approval of our resident a11y expert.
QA
General checklist